Fix: Exclude computed fields during pydantic MP serialization#845
Conversation
Signed-off-by: SkiHatDuckie <SkiHatDuckie@gmail.com>
|
Queued — the merge queue status continues in this comment ↓. |
dbutenhof
left a comment
dbutenhof
left a comment
There was a problem hiding this comment.
I suppose my concern would be if we're ever using this in a context where we want computed fields to be serialized (e.g., for a report) -- presumably that'd never be in a context where we're going to re-deserialize and encounter the problem, but this is one of those places where it's hard to get a full call dependency graph; and if Sam thinks this is safe, I'll throw on my +1 on the basis that, at least in isolation, the change looks OK.
Merge Queue Status
This pull request spent 28 minutes 23 seconds in the queue, including 28 minutes 4 seconds running CI. Required conditions to merge
|
Had this thought at first but the title/description is misleading. This only changes the encoding code which is used to pass objects from one process to another. |
sjmonson
changed the title
Yeah, that was the usage I found -- but there was a callback protocol that broke the call chain and I wasn't 100% certain we couldn't get there from somewhere else... though it seemed improbable. I'm reassured that you're comfortable with that diagnosis. |
…roject#845) ## Summary See the linked issue for more details. In short, fields with the`@computed_field` decorator are included in `model_dump` (and similar) by default, but are then treated as extra fields when passed to `model_validate`, leading to duplicates appearing in the final deserialized object. ## Details - Set `exclude_computed_fields=True` in `Serializer.to_dict_pydantic` and `.to_sequence_pydantic` - Remove pytest.mark.xfail from `test_encode_decode_generative` ## Test Plan `tox -e test-unit -- tests/unit/utils/test_encoding.py` ## Related Issues - Resolves vllm-project#446 - Alternative solution to vllm-project#448 --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [ ] Includes code generated or substantially modified by an AI agent - [ ] Includes tests generated or substantially modified by an AI agent > NOTE: the `Generated-by` or `Assisted-by` trailers should be used in git commit messages when code or tests were generated or substantially modified by an AI agent, as described in the project's [`DEVELOPING.md`](https://github.com/vllm-project/guidellm/blob/main/DEVELOPING.md) file. --- # git log commit 4c59acb Author: SkiHatDuckie <SkiHatDuckie@gmail.com> Date: Wed Jun 24 15:02:04 2026 -0400 Exclude computed fields during pydantic model dump Signed-off-by: SkiHatDuckie <SkiHatDuckie@gmail.com> --------- Signed-off-by: SkiHatDuckie <SkiHatDuckie@gmail.com>
3 participants
Summary
See the linked issue for more details. In short, fields with the
@computed_fielddecorator are included inmodel_dump(and similar) by default, but are then treated as extra fields when passed tomodel_validate, leading to duplicates appearing in the final deserialized object.Details
exclude_computed_fields=TrueinSerializer.to_dict_pydanticand.to_sequence_pydantictest_encode_decode_generativeTest Plan
tox -e test-unit -- tests/unit/utils/test_encoding.pyRelated Issues
Use of AI
git log
commit 4c59acb
Author: SkiHatDuckie SkiHatDuckie@gmail.com
Date: Wed Jun 24 15:02:04 2026 -0400
Signed-off-by: SkiHatDuckie SkiHatDuckie@gmail.com